Add test coverage to test-unit Makefile target #295
Conversation
carlory
requested review from
elevran,
kfirtoledo,
kfswain,
nilig,
nirrozenbaum and
shmuelk
as code owners
Signed-off-by: carlory <[email protected]>
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
| make test | ||
| - name: Upload coverage to Codecov | ||
| uses: codecov/codecov-action@18283e04ce6e62d37312384ff67231eb8fd56d24 # v5.4.3 |
There was a problem hiding this comment.
any reason we can't use a tagged release instead of a SHA?
There was a problem hiding this comment.
No special reason. I copied it from the project envoyproxy/ai-gateway: https://github.com/envoyproxy/ai-gateway/blob/main/.github/workflows/build_and_test.yaml#L70-L82
| - name: Upload coverage to Codecov | ||
| uses: codecov/codecov-action@18283e04ce6e62d37312384ff67231eb8fd56d24 # v5.4.3 | ||
| with: | ||
| fail_ci_if_error: true |
There was a problem hiding this comment.
what's considered an error for coverage?
There was a problem hiding this comment.
On error, exit with non-zero code
For example, if there is an issue with uploading the coverage file (e.g., network issues, invalid file paths, or errors with the Codecov service), the CI process will be marked as failed. This is helpful in environments where you want to ensure that coverage reports are always successfully uploaded to Codeco
| test-unit: download-tokenizer download-zmq | ||
| @printf "\033[33;1m==== Running Unit Tests ====\033[0m\n" | ||
| go test -ldflags="$(LDFLAGS)" -v ./... | ||
| go test -ldflags="$(LDFLAGS)" -v -covermode=atomic -coverprofile=go-test-coverage.out ./... |
There was a problem hiding this comment.
if we go with and pay the runtime penalty for covermode=atomic due to concurrent code execution, might be worthwhile to also enable -race detection in tests.
|
Hi, I was wondering about the rationale behind using an external service. What benefits would it bring? Since we already have the coverage file from go test, it seems possible to use tools like go tool cover—so I’m curious about which specific features we’re looking to leverage through CodeDev. |
|
@carlory are you still actively working on this? Let us know if you're still able/interested in pursuing this further. |
|
@elevran Sorry for the delay. I forgot to update this PR. I create a demo project in my account and try to add some changes to verify the codecov behavior because I'm not familiar with the codecov behavior. It seems that the repo/organization owner needs to configure this project in the codecov dashboard before this workflow can work. I'm unsure whether we want to use codecov in our project. If yes, I will update this PR after I check it on my demo project. |
@Jooho FYI: 
|
4 participants
Fix #287